fix(parquet/compress): avoid closing caller streams for uncompressed wrappers - #1003
Conversation
fallintoplace
changed the title
fallintoplace
changed the title
fallintoplace
changed the title
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
Verified against current main: nocodec.NewReader/NewWriter return the caller's stream directly when it already implements io.ReadCloser/io.WriteCloser, so calling Close() on the returned wrapper closes the caller's underlying stream. That is inconsistent with every other codec — gzip's Close does not touch the underlying stream, and snappy/brotli/zstd all wrap via io.NopCloser. The established contract for StreamingCodec is clearly that the codec does not take ownership of the caller's stream, so always wrapping in a no-op closer is the correct fix.
I confirmed no internal caller depends on the old close-through behavior. The tests are meaningful — they assert the underlying closeTrackingBuffer stays open after closing the wrapper. LGTM.
2 participants
The compressed streaming codecs close their own wrappers without taking ownership of the caller stream. The uncompressed codec passed through existing closers directly, so closing the wrapper also closed the underlying reader or writer.
This always wraps uncompressed streams in no-op closers and adds regression tests for both read and write wrappers.
Tests:
go test ./parquet/compress